Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add primary destructive button style #27774

Merged
merged 1 commit into from
Jan 5, 2021
Merged

Add primary destructive button style #27774

merged 1 commit into from
Jan 5, 2021

Conversation

hsingyuc
Copy link
Contributor

Description

Fix #27587
Add primary destructive button style

How has this been tested?

  1. npm run storybook: dev
  2. Go to isPrimaryDestructive button story
  3. Check if the button renders as expected
  4. Go to isDestructive and primary button stories, check there are no regressions

Screenshots

  1. Before

before

  1. After

after

  1. Hover

hover

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Dec 17, 2020
@talldan talldan added [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components Needs Design Feedback Needs general design feedback. labels Dec 17, 2020
@jasmussen
Copy link
Contributor

Thank you, nice PR!

If you are able to apply both a destructive and primary prop to the Button component, then it definitely follows that the visuals should account for that. So if we are to allow this, the change here seems good to me. The code looks good too, but I'd appreciate a sanity check.

@gziolo gziolo removed the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 5, 2021
gziolo
gziolo approved these changes Jan 5, 2021
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good too, but I'd appreciate a sanity check.

What type of check is necessary? Code-wise it's good to go

@jasmussen
Copy link
Contributor

What type of check is necessary? Code-wise it's good to go

That was the check!

Ship it.

@gziolo gziolo merged commit 6dc0b14 into WordPress:master Jan 5, 2021
@gziolo
Copy link
Member

gziolo commented Jan 5, 2021

Thank you for contributing 😃

@github-actions github-actions bot added this to the Gutenberg 9.8 milestone Jan 5, 2021
@hsingyuc hsingyuc deleted the fix/27587 branch January 5, 2021 21:02
@tomalec
Copy link
Contributor

tomalec commented Sep 9, 2021

@hsingyuc @gziolo Could you help me track what is the earliest version of @wordpress/components that has this fixed included? So I'd know if/when could I remove the workaround from our plugin?

@gziolo
Copy link
Member

gziolo commented Sep 10, 2021

@hsingyuc @gziolo Could you help me track what is the earliest version of @wordpress/components that has this fixed included? So I'd know if/when could I remove the workaround from our plugin?

It looks like it was published to npm in v12.0.2:
https://unpkg.com/browse/@wordpress/[email protected]/src/button/style.scss

You can use the UNPKG browser and see that it's missing in v12.0.1.

@eason9487 eason9487 mentioned this pull request Jan 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@ItsJonQ ItsJonQ Awaiting requested review from ItsJonQ

@jasmussen jasmussen Awaiting requested review from jasmussen

Assignees
No one assigned
Labels
[Feature] UI Components Impacts or related to the UI component system Needs Design Feedback Needs general design feedback. [Package] Components /packages/components
Projects
None yet
Milestone
Gutenberg 9.8
Development

Successfully merging this pull request may close these issues.

Button isDestructive isPrimary does not render Red background
5 participants
@hsingyuc @jasmussen @gziolo @tomalec @talldan